Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update pydantic to 1.10.17 #119430

Merged
merged 4 commits into from
Jun 21, 2024
Merged

Update pydantic to 1.10.17 #119430

merged 4 commits into from
Jun 21, 2024

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Jun 11, 2024 •
edited
Loading

Proposed change

https://github.com/pydantic/pydantic/releases/tag/v1.10.16
pydantic/pydantic@v1.10.15...v1.10.16
https://github.com/pydantic/pydantic/releases/tag/v1.10.17
pydantic/pydantic@v1.10.16...v1.10.17

https://github.com/home-assistant/core/actions/runs/9471830653
https://github.com/home-assistant/core/actions/runs/9599604587
https://wheels.home-assistant.io/musllinux/

Last bump: #116401

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@cdce8p cdce8p requested a review from a team as a code owner June 11, 2024 20:09
@home-assistant home-assistant bot added cla-signed core dependency small-pr PRs with less than 30 lines. labels Jun 11, 2024
@cdce8p cdce8p requested a review from bdraco June 11, 2024 20:10
@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

Looks like all the wheels are working

https://wheels.home-assistant.io/musllinux/

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

testing this now

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

It looks like there is a breaking change

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

The CI is failing as well

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

All the tests fail on the uiprotect library as well. It seems private attributes are broken in .16

Downgrading to 1.10.15 and they all pass again.

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

I looked at the pydantic docs and it seem like the uiprotect library is doing private attrs correctly but maybe not. Maybe you can spot an error there?

@cdce8p
Copy link
Member Author

cdce8p commented Jun 11, 2024

All the tests fail on the uiprotect library as well. It seems private attributes are broken in .16

Downgrading to 1.10.15 and they all pass again.

😮‍💨 They don't seem to get it right, do they? The plan as I know it, is to end support for v1 by the end of the month. Would be bad if it's still broken by then. Just wanted to fix a mypy error for the next release 😅

Unfortunately, it's a busy week for me. So I won't have the time to fix it myself.

@cdce8p
Copy link
Member Author

cdce8p commented Jun 11, 2024

The break is a result of the change here: pydantic/pydantic@5adc381#diff-a20bff027a309d77bb05981c6385eaed08dc7cbce12358c201076a4f1bf04733R13-R16

A minimal repo:

from pydantic import BaseModel

try:
    from pydantic.v1 import BaseModel
    from pydantic.v1.fields import PrivateAttr
except ImportError:
    from pydantic import BaseModel
    from pydantic.fields import PrivateAttr


class Model(BaseModel):
    _val: int | None = PrivateAttr(None)

m = Model()
m._val = 1

from pydantic.v1 import BaseModel doesn't raise an ImportError anymore. However, from pydantic.v1.fields import PrivateAttr seems to be broken. Replacing it with from pydantic.v1 import PrivateAttr works.

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

Thanks. Thats a bit annoying. We can change it in uiprotect for sure. I can do that after dinner.

I'm a bit worried it will be an issue for other libraries as well though

bdraco added a commit to uilibs/uiprotect that referenced this pull request Jun 11, 2024
@bdraco
fix: move PrivateAttr import to come from pydantic.v1
e26b67a 
related home-assistant/core#119430 (comment)
@bdraco bdraco mentioned this pull request Jun 11, 2024
Closed
@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

I attempted that change, but it looks like there is more breakage. It seems limited to python 3.12

uilibs/uiprotect#39

@bdraco
Copy link
Member

bdraco commented Jun 11, 2024

Feel free to push to my PR at uilibs/uiprotect#39. You should have access now

@epenet epenet marked this pull request as draft June 12, 2024 05:37
@epenet epenet mentioned this pull request Jun 12, 2024
Merged
22 tasks
@bdraco bdraco mentioned this pull request Jun 13, 2024
Closed
@bdraco bdraco mentioned this pull request Jun 15, 2024
Merged
@bdraco
Copy link
Member

bdraco commented Jun 15, 2024

Might be fixed by pydantic/pydantic#9660

@bdraco bdraco mentioned this pull request Jun 15, 2024
Merged
5 tasks
@adampetrovic adampetrovic mentioned this pull request Jun 19, 2024
Closed
1 task
@cdce8p cdce8p added the waiting-for-upstream We're waiting for a change upstream label Jun 19, 2024
@cdce8p cdce8p changed the title Update pydantic to 1.10.16 Update pydantic to 1.10.17 Jun 20, 2024
@cdce8p cdce8p mentioned this pull request Jun 20, 2024
Merged
20 tasks
@cdce8p cdce8p marked this pull request as ready for review June 20, 2024 17:13
bdraco
bdraco reviewed Jun 20, 2024
View reviewed changes
homeassistant/components/xbox/media_source.py
Comment on lines -8 to 9
from pydantic.error_wrappers import ValidationError
from pydantic import ValidationError
from xbox.webapi.api.client import XboxLiveClient
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change in pydantic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. Probably just an issue with pylint as pydantic is compiled. Somehow pylint isn't able to determine the module names properly.

Anyway, ValidationError is importable from both pydantic and pydantic.error_wrappers.

@cdce8p cdce8p added smash Indicator this PR is close to finish for merging or closing and removed waiting-for-upstream We're waiting for a change upstream labels Jun 20, 2024
@bdraco
Copy link
Member

bdraco commented Jun 20, 2024

Verified uiprotect CI is passing with this version as well.

@cdce8p cdce8p merged commit e5846fd into dev Jun 21, 2024
37 of 39 checks passed
@cdce8p cdce8p deleted the update-pydantic-1.10.16 branch June 21, 2024 08:16
@adampetrovic adampetrovic mentioned this pull request Jun 21, 2024
Closed
1 task
@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bdraco bdraco bdraco approved these changes

@hunterjm hunterjm Awaiting requested review from hunterjm hunterjm is a code owner

@epenet epenet Awaiting requested review from epenet epenet is a code owner

@robbiet480 robbiet480 Awaiting requested review from robbiet480 robbiet480 is a code owner

@frenck frenck Awaiting requested review from frenck frenck is a code owner

@bachya bachya Awaiting requested review from bachya bachya is a code owner

Assignees
No one assigned
Labels
cla-signed core dependency small-pr PRs with less than 30 lines. smash Indicator this PR is close to finish for merging or closing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cdce8p @bdraco